Limit releases to only show most recent ones #4570
Open
devnchill wants to merge 11 commits intoOWASP:mainfrom
Open
Limit releases to only show most recent ones #4570devnchill wants to merge 11 commits intoOWASP:mainfrom
devnchill wants to merge 11 commits intoOWASP:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary by CodeRabbit
WalkthroughAdds a client-side toggle to limit visible ChangesSnapshot releases truncation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
On contribution guidelines, it is mentioned that if pr implements new feature, will have to also write tests for it, so ig I don't have to be assigned again for tests and can instead write tests directly in this same pr? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/app/community/snapshots/`[id]/page.tsx:
- Around line 36-40: The effect that currently only sets showAllReleases to true
must reset the toggle when the incoming snapshot changes: update the useEffect
that watches snapshot so it sets setShowAllReleases(snapshot?.newReleases &&
snapshot.newReleases.length <= MAX_RELEASES_TO_SHOW) (or explicitly set true
when length <= MAX_RELEASES_TO_SHOW and false otherwise) so the
collapsed/expanded state is recomputed on each new snapshot; reference the
existing useEffect, snapshot, newReleases, MAX_RELEASES_TO_SHOW, and
setShowAllReleases symbols when making the change.
- Around line 107-110: The classname in the showButton object contains a typo
'darK:hover:text-white' that prevents Tailwind's dark variant from applying;
update the classname string in the showButton definition to use the correct
lowercase variant 'dark:hover:text-white' so the dark-mode hover style is
applied (locate the showButton constant and fix the classname property).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 918a8d62-d85b-4af0-ab90-fe466fbfb6c8
📒 Files selected for processing (1)
frontend/src/app/community/snapshots/[id]/page.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 4/5
- This PR is likely safe to merge, with minimal risk: the reported issue is a localized styling typo rather than a functional or data-impacting bug.
- In
frontend/src/app/community/snapshots/[id]/page.tsx,darK:hover:text-whiteshould bedark:hover:text-white; because Tailwind variants are case-sensitive, the dark-mode hover text color class is silently ignored. - User impact appears limited to dark-mode hover visuals on that page, so this is a minor UX regression rather than a merge-blocking defect.
- Pay close attention to
frontend/src/app/community/snapshots/[id]/page.tsx- fix thedark:variant casing to restore intended dark-mode hover styling.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/app/community/snapshots/[id]/page.tsx">
<violation number="1" location="frontend/src/app/community/snapshots/[id]/page.tsx:110">
P2: `darK:hover:text-white` has an uppercase `K` — Tailwind's `dark:` variant is case-sensitive and must be all-lowercase. This typo silently drops the dark-mode hover style.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
devnchill
dismissed stale reviews from cubic-dev-ai[bot] and coderabbitai[bot]
via
8df6804
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/components/SnapshotReleaseSection.tsx (2)
24-31: Remove the redundant cast and key fallback.
releaseis alreadyReleaseType, andRelease.idis required infrontend/src/types/release.ts, so theunknowncast and fallback key only hide contract regressions.Proposed cleanup
<Release - key={ - release.id || `${release.tagName}-${release.repositoryName ?? 'unknown'}-${index}` - } - release={release as unknown as ReleaseType} + key={release.id} + release={release} showAvatar={true} index={index} />Please verify with the existing TypeScript check after removing the cast.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/SnapshotReleaseSection.tsx` around lines 24 - 31, Remove the redundant type cast and fallback key: pass release directly (no "as unknown as ReleaseType") and use release.id as the key (remove the `${release.tagName}...` fallback), since ReleaseType and Release.id are required; update the JSX in the Release component invocation (the Release element, props `release` and `key`) and run the TypeScript check to verify there are no type errors.
12-15: Make the toggle state clearer for assistive tech.Use release-specific button text and expose the expanded state.
Proposed accessibility refinement
const showButton = { - label: showAll ? 'show less' : 'show all', + label: showAll ? 'Show less releases' : 'Show all releases', classname: 'dark:hover:text-white rounded-md border-1 font-light border-blue-400 p-2 text-blue-400 hover:bg-blue-500 hover:text-white', }- <button className={showButton.classname} type="button" onClick={onToggle}> + <button + aria-expanded={showAll} + className={showButton.classname} + type="button" + onClick={onToggle} + > {showButton.label} </button>Also applies to: 35-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/SnapshotReleaseSection.tsx` around lines 12 - 15, The toggle button in SnapshotReleaseSection currently uses a generic label and doesn't expose expanded state; update the showButton logic (and the similar button block around lines 35-39) to produce release-specific labels (e.g., include the release name or version via the component prop/state used in SnapshotReleaseSection) and ensure the rendered button exposes its expanded state for assistive tech by adding aria-expanded={showAll} and an aria-controls that points to the collapsible content's id; adjust classname generation only as needed and keep the same button element (refer to showButton and showAll) so screen readers read distinct, contextual labels and know when the panel is expanded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/components/SnapshotReleaseSection.tsx`:
- Around line 24-31: Remove the redundant type cast and fallback key: pass
release directly (no "as unknown as ReleaseType") and use release.id as the key
(remove the `${release.tagName}...` fallback), since ReleaseType and Release.id
are required; update the JSX in the Release component invocation (the Release
element, props `release` and `key`) and run the TypeScript check to verify there
are no type errors.
- Around line 12-15: The toggle button in SnapshotReleaseSection currently uses
a generic label and doesn't expose expanded state; update the showButton logic
(and the similar button block around lines 35-39) to produce release-specific
labels (e.g., include the release name or version via the component prop/state
used in SnapshotReleaseSection) and ensure the rendered button exposes its
expanded state for assistive tech by adding aria-expanded={showAll} and an
aria-controls that points to the collapsible content's id; adjust classname
generation only as needed and keep the same button element (refer to showButton
and showAll) so screen readers read distinct, contextual labels and know when
the panel is expanded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d6d748d3-9647-4744-85bf-f369199df8a2
📒 Files selected for processing (2)
frontend/src/app/community/snapshots/[id]/page.tsxfrontend/src/components/SnapshotReleaseSection.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/community/snapshots/[id]/page.tsx
devnchill
dismissed stale reviews from cubic-dev-ai[bot] and coderabbitai[bot]
via
73e74da
This makes it easy to test this component
devnchill
force-pushed
the
feat/limit-release
branch
from
73e74da to
102607c
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4570 +/- ##
==========================================
- Coverage 98.92% 98.92% -0.01%
==========================================
Files 527 528 +1
Lines 16956 16964 +8
Branches 2360 2413 +53
==========================================
+ Hits 16774 16781 +7
- Misses 97 98 +1
Partials 85 85
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/src/app/community/snapshots/[id]/page.tsx (1)
34-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset on
snapshotKey, notsnapshotobject identity.At Line 36, using
snapshotin the dependency array can collapse the list again when Apollo emits a refreshed object for the same snapshot. Keying this effect tosnapshotKeyavoids unexpected UI resets.Proposed fix
useEffect(() => { setShowAllReleases(false) - }, [snapshot]) + }, [snapshotKey])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/community/snapshots/`[id]/page.tsx around lines 34 - 36, The useEffect currently resets the UI based on the full snapshot object identity; change its dependency to the stable snapshot identifier instead so UI doesn't reset on refreshed but same snapshot instances—update the effect that calls setShowAllReleases(false) in the useEffect whose dependency is snapshot to depend on snapshotKey (the stable snapshot id) rather than snapshot.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/__tests__/unit/components/SnapshotReleaseSection.test.tsx`:
- Around line 29-41: Add a regression test in SnapshotReleaseSection.test.tsx
that verifies the full expand/collapse flow for the ReleasesSection component:
render <ReleasesSection releases={mockReleases} showAll={false}
onToggle={onToggle} />, click the "Show more" button (using getByRole with name
'Show more'), assert the UI updates to show the collapsed state toggled (e.g.,
the button label changes to 'Hide' or the expanded items are visible), then
click the toggle again and assert the UI returns to the original collapsed state
and onToggle was called for each click (expect onToggle
toHaveBeenCalledTimes(2)); reference the ReleasesSection component, the showAll
prop, the onToggle handler, and the "Show more"/"Hide" button labels to locate
and implement the checks.
In `@frontend/src/components/SnapshotReleaseSection.tsx`:
- Line 23: The prop cast bypasses TypeScript safety—remove the unnecessary
double-cast and pass the value directly to the Release component: change the JSX
prop currently using "release as unknown as ReleaseType" to simply "release";
also ensure the iterated variable is declared/typed as ReleaseType (from
releases: ReleaseType[]) so the Release component (prop: release) receives the
correct type without casting.
---
Duplicate comments:
In `@frontend/src/app/community/snapshots/`[id]/page.tsx:
- Around line 34-36: The useEffect currently resets the UI based on the full
snapshot object identity; change its dependency to the stable snapshot
identifier instead so UI doesn't reset on refreshed but same snapshot
instances—update the effect that calls setShowAllReleases(false) in the
useEffect whose dependency is snapshot to depend on snapshotKey (the stable
snapshot id) rather than snapshot.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 06e11e68-ed4b-449e-a23d-845c5081218b
📒 Files selected for processing (4)
frontend/__tests__/unit/components/SnapshotReleaseSection.test.tsxfrontend/src/app/community/snapshots/[id]/page.tsxfrontend/src/components/SnapshotReleaseSection.tsxfrontend/src/utils/constants.ts
Comment on lines
+29
to
+41
| it('renders the show more button for releases > MAX_RELEASES_TO_SHOW', () => { | ||
| render(<ReleasesSection releases={mockReleases} showAll={false} onToggle={jest.fn()} />) | ||
| expect(screen.getByRole('button', { name: 'Show more' })).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('calls onToggle when the show more button is clicked', () => { | ||
| const onToggle = jest.fn() | ||
|
|
||
| render(<ReleasesSection releases={mockReleases} showAll={false} onToggle={onToggle} />) | ||
|
|
||
| fireEvent.click(screen.getByRole('button', { name: 'Show more' })) | ||
| expect(onToggle).toHaveBeenCalledTimes(1) | ||
| }) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add a regression test for the full expand/collapse flow.
Current cases assert initial render and callback invocation, but not that the UI can return from expanded state (the core toggle contract).
Suggested test addition
+ it('supports toggling between collapsed and expanded states', () => {
+ const onToggle = jest.fn()
+ const { rerender } = render(
+ <ReleasesSection releases={mockReleases} showAll={false} onToggle={onToggle} />
+ )
+
+ expect(screen.getAllByText(/Release v/)).toHaveLength(MAX_RELEASES_TO_SHOW)
+
+ rerender(<ReleasesSection releases={mockReleases} showAll={true} onToggle={onToggle} />)
+ expect(screen.getAllByText(/Release v/)).toHaveLength(MAX_RELEASES_TO_SHOW + 1)
+ expect(screen.getByRole('button')).toBeInTheDocument()
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('renders the show more button for releases > MAX_RELEASES_TO_SHOW', () => { | |
| render(<ReleasesSection releases={mockReleases} showAll={false} onToggle={jest.fn()} />) | |
| expect(screen.getByRole('button', { name: 'Show more' })).toBeInTheDocument() | |
| }) | |
| it('calls onToggle when the show more button is clicked', () => { | |
| const onToggle = jest.fn() | |
| render(<ReleasesSection releases={mockReleases} showAll={false} onToggle={onToggle} />) | |
| fireEvent.click(screen.getByRole('button', { name: 'Show more' })) | |
| expect(onToggle).toHaveBeenCalledTimes(1) | |
| }) | |
| it('renders the show more button for releases > MAX_RELEASES_TO_SHOW', () => { | |
| render(<ReleasesSection releases={mockReleases} showAll={false} onToggle={jest.fn()} />) | |
| expect(screen.getByRole('button', { name: 'Show more' })).toBeInTheDocument() | |
| }) | |
| it('calls onToggle when the show more button is clicked', () => { | |
| const onToggle = jest.fn() | |
| render(<ReleasesSection releases={mockReleases} showAll={false} onToggle={onToggle} />) | |
| fireEvent.click(screen.getByRole('button', { name: 'Show more' })) | |
| expect(onToggle).toHaveBeenCalledTimes(1) | |
| }) | |
| it('supports toggling between collapsed and expanded states', () => { | |
| const onToggle = jest.fn() | |
| const { rerender } = render( | |
| <ReleasesSection releases={mockReleases} showAll={false} onToggle={onToggle} /> | |
| ) | |
| expect(screen.getAllByText(/Release v/)).toHaveLength(MAX_RELEASES_TO_SHOW) | |
| rerender(<ReleasesSection releases={mockReleases} showAll={true} onToggle={onToggle} />) | |
| expect(screen.getAllByText(/Release v/)).toHaveLength(MAX_RELEASES_TO_SHOW + 1) | |
| expect(screen.getByRole('button')).toBeInTheDocument() | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/__tests__/unit/components/SnapshotReleaseSection.test.tsx` around
lines 29 - 41, Add a regression test in SnapshotReleaseSection.test.tsx that
verifies the full expand/collapse flow for the ReleasesSection component: render
<ReleasesSection releases={mockReleases} showAll={false} onToggle={onToggle} />,
click the "Show more" button (using getByRole with name 'Show more'), assert the
UI updates to show the collapsed state toggled (e.g., the button label changes
to 'Hide' or the expanded items are visible), then click the toggle again and
assert the UI returns to the original collapsed state and onToggle was called
for each click (expect onToggle toHaveBeenCalledTimes(2)); reference the
ReleasesSection component, the showAll prop, the onToggle handler, and the "Show
more"/"Hide" button labels to locate and implement the checks.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/SnapshotReleaseSection.tsx`:
- Line 30: ShowMoreButton's internal isExpanded can get out of sync with the
parent's showAll (causing "Show less" to appear when only MAX_RELEASES_TO_SHOW
items are shown) and the button is missing an accessibility attribute; fix by
wiring the parent showAll into the component lifecycle (either pass showAll into
ShowMoreButton and use it to reset internal state inside ShowMoreButton or
render <ShowMoreButton key={showAll} .../> so it remounts when showAll changes),
ensure onToggle is still called to notify the parent, and add
aria-expanded={isExpanded} to the underlying <Button> element inside
ShowMoreButton so the expanded state is exposed to assistive tech.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bdb4505b-8dfe-4496-abfc-c38ad79fa79d
📒 Files selected for processing (1)
frontend/src/components/SnapshotReleaseSection.tsx
| ) | ||
| })} | ||
| </div> | ||
| {releases.length > MAX_RELEASES_TO_SHOW && <ShowMoreButton onToggle={onToggle} />} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify ShowMoreButton API and whether it receives expansion state.
fd -t f "ShowMoreButton.tsx" "ShowMoreButton.jsx" "ShowMoreButton/index.tsx" "ShowMoreButton/index.jsx"
rg -n -C3 "type .*ShowMoreButton|interface .*ShowMoreButton|function ShowMoreButton|const ShowMoreButton|aria-expanded|Show all|Show less|Show more|onToggle" --type ts --type tsx --type js --type jsx
# Verify call sites and passed props.
rg -n -C2 "<ShowMoreButton" --type tsx --type jsxRepository: OWASP/Nest
Length of output: 358
🏁 Script executed:
#!/bin/bash
# Find ShowMoreButton component file
fd "ShowMoreButton" --type f
# Check the ShowMoreButton implementation
fd "ShowMoreButton" --type f -x cat -n {} \;Repository: OWASP/Nest
Length of output: 2367
🏁 Script executed:
#!/bin/bash
# Find and examine SnapshotReleaseSection.tsx
fd "SnapshotReleaseSection" --type f -x cat -n {} \;Repository: OWASP/Nest
Length of output: 3235
🏁 Script executed:
#!/bin/bash
# Find where ReleasesSection is used/imported
rg "ReleasesSection" --type ts --type tsxRepository: OWASP/Nest
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Find where ReleasesSection is used
rg "ReleasesSection" -g "*.ts" -g "*.tsx" -g "*.js" -g "*.jsx"Repository: OWASP/Nest
Length of output: 1353
🏁 Script executed:
#!/bin/bash
# Examine the page component where ReleasesSection is used
cat -n "frontend/src/app/community/snapshots/[id]/page.tsx"Repository: OWASP/Nest
Length of output: 7942
ShowMoreButton needs to synchronize with parent showAll state and include aria-expanded attribute.
ShowMoreButton maintains internal isExpanded state that becomes out of sync when parent's showAll resets (e.g., when snapshot changes). This causes the button text to display "Show less" while only showing MAX_RELEASES_TO_SHOW items. Additionally, the button lacks an aria-expanded attribute for proper accessibility.
Consider either:
- Passing
showAllprop toShowMoreButtonso it can reset when the parent state resets, or - Using a React
keyonShowMoreButtonto force remount whenshowAllchanges
Add aria-expanded={isExpanded} to the <Button> element for accessibility.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/SnapshotReleaseSection.tsx` at line 30,
ShowMoreButton's internal isExpanded can get out of sync with the parent's
showAll (causing "Show less" to appear when only MAX_RELEASES_TO_SHOW items are
shown) and the button is missing an accessibility attribute; fix by wiring the
parent showAll into the component lifecycle (either pass showAll into
ShowMoreButton and use it to reset internal state inside ShowMoreButton or
render <ShowMoreButton key={showAll} .../> so it remounts when showAll changes),
ensure onToggle is still called to notify the parent, and add
aria-expanded={isExpanded} to the underlying <Button> element inside
ShowMoreButton so the expanded state is exposed to assistive tech.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #4560
Earlier the snapshots page would display all release at once making the ui look overwhelming.
This pr:
9most recent releasesshow allbutton which on clicking would show all the releasesshow lesswhich would again collapse it to show 9 most recent ones.Checklist
make check-testlocally: all warnings addressed, tests passed